Skip to content

Conversation

cor3ntin
Copy link
Contributor

@cor3ntin cor3ntin commented May 28, 2025

In the standard, constraint satisfaction checking is done on the normalized form of a constraint.

Clang instead substitutes on the non-normalized form, which causes us to report substitution failures in template arguments or concept ids, which is non-conforming but unavoidable without a parameter mapping

This patch normalizes before satisfaction checking. However, we preserve concept-id nodes in the normalized form, solely for diagnostics purposes.

This addresses #61811 and related concepts conformance bugs, ideally to make the remaining implementation of concept template parameters easier

Fixes #135190
Fixes #61811

Co-authored-by: Younan Zhang [email protected]

@cor3ntin cor3ntin requested review from erichkeane and zyn0217 May 28, 2025 14:33
Copy link

github-actions bot commented Jun 1, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@cor3ntin cor3ntin force-pushed the corentin/use_normalization_for_satisfaction branch 3 times, most recently from f29061c to 0b7a92a Compare June 8, 2025 08:50
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to make another pass later.

/// substituted constraint expr, if the template arguments could be
/// substituted into them, or a diagnostic if substitution resulted in
/// an invalid expression.
using UnsatisfiedConstraintRecord =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Record is such an overloaded term, can we find a better term? I feel like UnsatisfiedConstraintUnion would almost be better here, maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is pre-existing, we could change it afterwards

return getTrailingObjects() + NumRecords;
}

ArrayRef<UnsatisfiedConstraintRecord> records() const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, can we get a more descriptive name for this member function. Especially because searching for this name will hit so many other things on a naive search.

struct alignas(ConstraintAlignment) FoldExpandedConstraint;
public:
ConstraintKind getKind() const {
return static_cast<ConstraintKind>(Atomic.Kind);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth commenting this is only valid due to initial common sequence.

@cor3ntin cor3ntin marked this pull request as ready for review June 26, 2025 10:04
@cor3ntin cor3ntin requested a review from Endilll as a code owner June 26, 2025 10:04
@cor3ntin
Copy link
Contributor Author

There are still a few test failures, including in libc++, but I think we are are at the stage where this could benefit from more eyes

@cor3ntin cor3ntin force-pushed the corentin/use_normalization_for_satisfaction branch from 2da1c7f to a08930e Compare August 11, 2025 10:21
@cor3ntin cor3ntin force-pushed the corentin/use_normalization_for_satisfaction branch from 141cca5 to e04b68f Compare September 9, 2025 14:08
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules HLSL HLSL Language Support labels Sep 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 9, 2025

@llvm/pr-subscribers-libcxx
@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-hlsl

Author: Corentin Jabot (cor3ntin)

Changes

In the standard, constraint satisfaction checking is done on the normalized form of a constraint.

Clang instead substitutes on the non-normalized form, which causes us to report substitution failures in template arguments or concept ids, which is non-conforming but unavoidable without a parameter mapping

This patch normalizes before satisfaction checking. However, we preserve concept-id nodes in the normalized form, solely for diagnostics purposes.

This is a very incomplete attempt at addressing #61811 and related concepts related conformance bugs, ideally to make the implementation of concept template parameters easier

There is still ~3 failing test files

Fixes #135190
Fixes #61811

Co-authored-by: Younan Zhang <[email protected]>


Patch is 258.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141776.diff

49 Files Affected:

  • (modified) clang/include/clang/AST/ASTConcept.h (+20-9)
  • (modified) clang/include/clang/AST/ASTContext.h (+43)
  • (modified) clang/include/clang/AST/TemplateBase.h (-1)
  • (modified) clang/include/clang/Sema/Sema.h (+43-41)
  • (modified) clang/include/clang/Sema/SemaConcept.h (+335-78)
  • (modified) clang/include/clang/Sema/Template.h (+22-6)
  • (modified) clang/lib/AST/ASTConcept.cpp (+23-4)
  • (modified) clang/lib/AST/ASTContext.cpp (+1-21)
  • (modified) clang/lib/AST/ASTImporter.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaConcept.cpp (+1275-707)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+7-5)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+10-6)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+2-1)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+75-33)
  • (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+25-12)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+155-79)
  • (modified) clang/lib/Sema/TreeTransform.h (+15-10)
  • (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+1-1)
  • (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+7-2)
  • (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+13-6)
  • (modified) clang/test/AST/ast-dump-concepts.cpp (+6-4)
  • (modified) clang/test/AST/ast-dump-ctad-alias.cpp (+11-10)
  • (modified) clang/test/CXX/drs/cwg25xx.cpp (+8-6)
  • (modified) clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp (+2-1)
  • (modified) clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp (+7-7)
  • (modified) clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp (+15-20)
  • (modified) clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp (+2-2)
  • (modified) clang/test/CXX/expr/expr.prim/expr.prim.req/type-requirement.cpp (+6-6)
  • (modified) clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp (+2-3)
  • (modified) clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp (+42-17)
  • (modified) clang/test/CXX/temp/temp.param/p10-2a.cpp (+13-10)
  • (modified) clang/test/SemaCXX/cxx23-assume.cpp (+4-5)
  • (modified) clang/test/SemaCXX/cxx2b-deducing-this.cpp (+4-4)
  • (modified) clang/test/SemaCXX/cxx2c-fold-exprs.cpp (+162-40)
  • (modified) clang/test/SemaCXX/cxx2c-template-template-param.cpp (+2-2)
  • (modified) clang/test/SemaCXX/invalid-requirement-requires-expr.cpp (+3-4)
  • (modified) clang/test/SemaCXX/overload-resolution-deferred-templates.cpp (+1-2)
  • (modified) clang/test/SemaCXX/type-traits.cpp (+2-2)
  • (modified) clang/test/SemaHLSL/BuiltIns/Buffers.hlsl (+3-3)
  • (modified) clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl (+3-3)
  • (modified) clang/test/SemaTemplate/concepts-recovery-expr.cpp (+21-11)
  • (modified) clang/test/SemaTemplate/concepts-recursive-inst.cpp (+14-13)
  • (modified) clang/test/SemaTemplate/concepts.cpp (+11-3)
  • (modified) clang/test/SemaTemplate/deduction-guide.cpp (+9-6)
  • (modified) clang/test/SemaTemplate/instantiate-abbreviated-template.cpp (+1)
  • (modified) clang/test/SemaTemplate/instantiate-expanded-type-constraint.cpp (+2-2)
  • (modified) clang/test/SemaTemplate/instantiate-requires-expr.cpp (+15-5)
  • (modified) clang/test/SemaTemplate/instantiate-template-argument.cpp (+9-5)
  • (modified) clang/test/SemaTemplate/pr52970.cpp (+1-1)
diff --git a/clang/include/clang/AST/ASTConcept.h b/clang/include/clang/AST/ASTConcept.h
index 72da0059744f2..9babfd39dd668 100644
--- a/clang/include/clang/AST/ASTConcept.h
+++ b/clang/include/clang/AST/ASTConcept.h
@@ -28,13 +28,23 @@ namespace clang {
 
 class ConceptDecl;
 class TemplateDecl;
+class ConceptReference;
 class Expr;
 class NamedDecl;
 struct PrintingPolicy;
 
+/// Pairs of unsatisfied atomic constraint expressions along with the
+/// substituted constraint expr, if the template arguments could be
+/// substituted into them, or a diagnostic if substitution resulted in
+/// an invalid expression.
+using UnsatisfiedConstraintRecord =
+    llvm::PointerUnion<const Expr *, const ConceptReference *,
+                       std::pair<SourceLocation, StringRef> *>;
+
 /// The result of a constraint satisfaction check, containing the necessary
 /// information to diagnose an unsatisfied constraint.
 class ConstraintSatisfaction : public llvm::FoldingSetNode {
+private:
   // The template-like entity that 'owns' the constraint checked here (can be a
   // constrained entity or a concept).
   const NamedDecl *ConstraintOwner = nullptr;
@@ -49,7 +59,6 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
       : ConstraintOwner(ConstraintOwner), TemplateArgs(TemplateArgs) {}
 
   using SubstitutionDiagnostic = std::pair<SourceLocation, StringRef>;
-  using Detail = llvm::PointerUnion<Expr *, SubstitutionDiagnostic *>;
 
   bool IsSatisfied = false;
   bool ContainsErrors = false;
@@ -57,7 +66,7 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
   /// \brief The substituted constraint expr, if the template arguments could be
   /// substituted into them, or a diagnostic if substitution resulted in an
   /// invalid expression.
-  llvm::SmallVector<Detail, 4> Details;
+  llvm::SmallVector<UnsatisfiedConstraintRecord, 4> Details;
 
   void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C) {
     Profile(ID, C, ConstraintOwner, TemplateArgs);
@@ -75,13 +84,6 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
   }
 };
 
-/// Pairs of unsatisfied atomic constraint expressions along with the
-/// substituted constraint expr, if the template arguments could be
-/// substituted into them, or a diagnostic if substitution resulted in
-/// an invalid expression.
-using UnsatisfiedConstraintRecord =
-    llvm::PointerUnion<Expr *, std::pair<SourceLocation, StringRef> *>;
-
 /// \brief The result of a constraint satisfaction check, containing the
 /// necessary information to diagnose an unsatisfied constraint.
 ///
@@ -101,6 +103,10 @@ struct ASTConstraintSatisfaction final :
     return getTrailingObjects() + NumRecords;
   }
 
+  ArrayRef<UnsatisfiedConstraintRecord> records() const {
+    return {begin(), end()};
+  }
+
   ASTConstraintSatisfaction(const ASTContext &C,
                             const ConstraintSatisfaction &Satisfaction);
   ASTConstraintSatisfaction(const ASTContext &C,
@@ -282,6 +288,11 @@ class TypeConstraint {
   }
 };
 
+/// Insertion operator for diagnostics.  This allows sending TemplateName's
+/// into a diagnostic with <<.
+const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+                                      const ConceptReference *C);
+
 } // clang
 
 #endif // LLVM_CLANG_AST_ASTCONCEPT_H
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 1c17333b722f8..0f808cbe2c17c 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -51,6 +51,27 @@ class FixedPointSemantics;
 struct fltSemantics;
 template <typename T, unsigned N> class SmallPtrSet;
 
+template <> struct DenseMapInfo<llvm::FoldingSetNodeID> {
+  static FoldingSetNodeID getEmptyKey() { return FoldingSetNodeID{}; }
+
+  static FoldingSetNodeID getTombstoneKey() {
+    FoldingSetNodeID id;
+    for (size_t i = 0; i < sizeof(id) / sizeof(unsigned); ++i) {
+      id.AddInteger(std::numeric_limits<unsigned>::max());
+    }
+    return id;
+  }
+
+  static unsigned getHashValue(const FoldingSetNodeID &Val) {
+    return Val.ComputeHash();
+  }
+
+  static bool isEqual(const FoldingSetNodeID &LHS,
+                      const FoldingSetNodeID &RHS) {
+    return LHS == RHS;
+  }
+};
+
 } // namespace llvm
 
 namespace clang {
@@ -3852,4 +3873,26 @@ typename clang::LazyGenerationalUpdatePtr<Owner, T, Update>::ValueType
   return Value;
 }
 
+template <> struct llvm::DenseMapInfo<llvm::FoldingSetNodeID> {
+  static FoldingSetNodeID getEmptyKey() { return FoldingSetNodeID{}; }
+
+  static FoldingSetNodeID getTombstoneKey() {
+    FoldingSetNodeID id;
+    for (size_t i = 0; i < sizeof(id) / sizeof(unsigned); ++i) {
+      id.AddInteger(std::numeric_limits<unsigned>::max());
+    }
+    return id;
+  }
+
+  static unsigned getHashValue(const FoldingSetNodeID &Val) {
+    return Val.ComputeHash();
+  }
+
+  static bool isEqual(const FoldingSetNodeID &LHS,
+                      const FoldingSetNodeID &RHS) {
+    return LHS == RHS;
+  }
+};
+
+
 #endif // LLVM_CLANG_AST_ASTCONTEXT_H
diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h
index de248ac3cf703..0359f4beb6309 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -475,7 +475,6 @@ class TemplateArgument {
   /// Used to insert TemplateArguments into FoldingSets.
   void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context) const;
 };
-
 /// Location information for a TemplateArgument.
 struct TemplateArgumentLocInfo {
   struct TemplateTemplateArgLocInfo {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 88b67eed5fd37..a508d33018292 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -65,6 +65,7 @@
 #include "clang/Sema/Redeclaration.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/SemaBase.h"
+#include "clang/Sema/SemaConcept.h"
 #include "clang/Sema/TypoCorrection.h"
 #include "clang/Sema/Weak.h"
 #include "llvm/ADT/APInt.h"
@@ -11688,8 +11689,9 @@ class Sema final : public SemaBase {
   ExprResult
   CheckConceptTemplateId(const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
                          const DeclarationNameInfo &ConceptNameInfo,
-                         NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
-                         const TemplateArgumentListInfo *TemplateArgs);
+                         NamedDecl *FoundDecl, TemplateDecl *NamedConcept,
+                         const TemplateArgumentListInfo *TemplateArgs,
+                         bool DoCheckConstraintSatisfaction = true);
 
   void diagnoseMissingTemplateArguments(TemplateName Name, SourceLocation Loc);
   void diagnoseMissingTemplateArguments(const CXXScopeSpec &SS,
@@ -12002,6 +12004,13 @@ class Sema final : public SemaBase {
                                  bool UpdateArgsWithConversions = true,
                                  bool *ConstraintsNotSatisfied = nullptr);
 
+  bool CheckTemplateArgumentList(
+      TemplateDecl *Template, TemplateParameterList *Params,
+      SourceLocation TemplateLoc, TemplateArgumentListInfo &TemplateArgs,
+      const DefaultArguments &DefaultArgs, bool PartialTemplateArgs,
+      CheckTemplateArgumentInfo &CTAI, bool UpdateArgsWithConversions = true,
+      bool *ConstraintsNotSatisfied = nullptr);
+
   bool CheckTemplateTypeArgument(
       TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
       SmallVectorImpl<TemplateArgument> &SugaredConverted,
@@ -12776,6 +12785,9 @@ class Sema final : public SemaBase {
   void MarkUsedTemplateParameters(ArrayRef<TemplateArgument> TemplateArgs,
                                   unsigned Depth, llvm::SmallBitVector &Used);
 
+  void MarkUsedTemplateParameters(ArrayRef<TemplateArgumentLoc> TemplateArgs,
+                                  unsigned Depth, llvm::SmallBitVector &Used);
+
   void
   MarkDeducedTemplateParameters(const FunctionTemplateDecl *FunctionTemplate,
                                 llvm::SmallBitVector &Deduced) {
@@ -13073,6 +13085,8 @@ class Sema final : public SemaBase {
     /// Whether we're substituting into constraints.
     bool InConstraintSubstitution;
 
+    bool InParameterMappingSubstitution;
+
     /// The point of instantiation or synthesis within the source code.
     SourceLocation PointOfInstantiation;
 
@@ -13338,6 +13352,11 @@ class Sema final : public SemaBase {
                          const MultiLevelTemplateArgumentList &TemplateArgs,
                          TemplateArgumentListInfo &Outputs);
 
+  bool SubstTemplateArgumentsInParameterMapping(
+      ArrayRef<TemplateArgumentLoc> Args, SourceLocation BaseLoc,
+      const MultiLevelTemplateArgumentList &TemplateArgs,
+      TemplateArgumentListInfo &Out, bool BuildPackExpansionTypes);
+
   /// Retrieve the template argument list(s) that should be used to
   /// instantiate the definition of the given declaration.
   ///
@@ -13799,6 +13818,12 @@ class Sema final : public SemaBase {
            CodeSynthesisContexts.back().InConstraintSubstitution;
   }
 
+  bool inParameterMappingSubstitution() const {
+    return !CodeSynthesisContexts.empty() &&
+           CodeSynthesisContexts.back().InParameterMappingSubstitution &&
+           !inConstraintSubstitution();
+  }
+
   using EntityPrinter = llvm::function_ref<void(llvm::raw_ostream &)>;
 
   /// \brief create a Requirement::SubstitutionDiagnostic with only a
@@ -14683,6 +14708,10 @@ class Sema final : public SemaBase {
     SatisfactionStack.swap(NewSS);
   }
 
+  using ConstrainedDeclOrNestedRequirement =
+      llvm::PointerUnion<const NamedDecl *,
+                         const concepts::NestedRequirement *>;
+
   /// Check whether the given expression is a valid constraint expression.
   /// A diagnostic is emitted if it is not, false is returned, and
   /// PossibleNonPrimary will be set to true if the failure might be due to a
@@ -14707,44 +14736,12 @@ class Sema final : public SemaBase {
   /// \returns true if an error occurred and satisfaction could not be checked,
   /// false otherwise.
   bool CheckConstraintSatisfaction(
-      const NamedDecl *Template,
+      ConstrainedDeclOrNestedRequirement Entity,
       ArrayRef<AssociatedConstraint> AssociatedConstraints,
       const MultiLevelTemplateArgumentList &TemplateArgLists,
-      SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction) {
-    llvm::SmallVector<Expr *, 4> Converted;
-    return CheckConstraintSatisfaction(Template, AssociatedConstraints,
-                                       Converted, TemplateArgLists,
-                                       TemplateIDRange, Satisfaction);
-  }
-
-  /// \brief Check whether the given list of constraint expressions are
-  /// satisfied (as if in a 'conjunction') given template arguments.
-  /// Additionally, takes an empty list of Expressions which is populated with
-  /// the instantiated versions of the ConstraintExprs.
-  /// \param Template the template-like entity that triggered the constraints
-  /// check (either a concept or a constrained entity).
-  /// \param ConstraintExprs a list of constraint expressions, treated as if
-  /// they were 'AND'ed together.
-  /// \param ConvertedConstraints a out parameter that will get populated with
-  /// the instantiated version of the ConstraintExprs if we successfully checked
-  /// satisfaction.
-  /// \param TemplateArgList the multi-level list of template arguments to
-  /// substitute into the constraint expression. This should be relative to the
-  /// top-level (hence multi-level), since we need to instantiate fully at the
-  /// time of checking.
-  /// \param TemplateIDRange The source range of the template id that
-  /// caused the constraints check.
-  /// \param Satisfaction if true is returned, will contain details of the
-  /// satisfaction, with enough information to diagnose an unsatisfied
-  /// expression.
-  /// \returns true if an error occurred and satisfaction could not be checked,
-  /// false otherwise.
-  bool CheckConstraintSatisfaction(
-      const NamedDecl *Template,
-      ArrayRef<AssociatedConstraint> AssociatedConstraints,
-      llvm::SmallVectorImpl<Expr *> &ConvertedConstraints,
-      const MultiLevelTemplateArgumentList &TemplateArgList,
-      SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction);
+      SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction,
+      const ConceptReference *TopLevelConceptId = nullptr,
+      Expr **ConvertedExpr = nullptr);
 
   /// \brief Check whether the given non-dependent constraint expression is
   /// satisfied. Returns false and updates Satisfaction with the satisfaction
@@ -14810,16 +14807,17 @@ class Sema final : public SemaBase {
   /// \param First whether this is the first time an unsatisfied constraint is
   /// diagnosed for this error.
   void DiagnoseUnsatisfiedConstraint(const ConstraintSatisfaction &Satisfaction,
+                                     SourceLocation Loc = {},
                                      bool First = true);
 
   /// \brief Emit diagnostics explaining why a constraint expression was deemed
   /// unsatisfied.
   void
-  DiagnoseUnsatisfiedConstraint(const ASTConstraintSatisfaction &Satisfaction,
+  DiagnoseUnsatisfiedConstraint(const ConceptSpecializationExpr *ConstraintExpr,
                                 bool First = true);
 
   const NormalizedConstraint *getNormalizedAssociatedConstraints(
-      const NamedDecl *ConstrainedDecl,
+      ConstrainedDeclOrNestedRequirement Entity,
       ArrayRef<AssociatedConstraint> AssociatedConstraints);
 
   /// \brief Check whether the given declaration's associated constraints are
@@ -14844,6 +14842,9 @@ class Sema final : public SemaBase {
       const NamedDecl *D1, ArrayRef<AssociatedConstraint> AC1,
       const NamedDecl *D2, ArrayRef<AssociatedConstraint> AC2);
 
+  llvm::DenseMap<llvm::FoldingSetNodeID, CachedConceptIdConstraint>
+      ConceptIdSatisfactionCache;
+
 private:
   /// Caches pairs of template-like decls whose associated constraints were
   /// checked for subsumption and whether or not the first's constraints did in
@@ -14854,7 +14855,8 @@ class Sema final : public SemaBase {
   /// constrained declarations). If an error occurred while normalizing the
   /// associated constraints of the template or concept, nullptr will be cached
   /// here.
-  llvm::DenseMap<const NamedDecl *, NormalizedConstraint *> NormalizationCache;
+  llvm::DenseMap<ConstrainedDeclOrNestedRequirement, NormalizedConstraint *>
+      NormalizationCache;
 
   llvm::ContextualFoldingSet<ConstraintSatisfaction, const ASTContext &>
       SatisfactionCache;
diff --git a/clang/include/clang/Sema/SemaConcept.h b/clang/include/clang/Sema/SemaConcept.h
index 648a9c51ae6c1..3ba48b75c1286 100644
--- a/clang/include/clang/Sema/SemaConcept.h
+++ b/clang/include/clang/Sema/SemaConcept.h
@@ -16,130 +16,387 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/UnsignedOrNone.h"
+#include "clang/Sema/Ownership.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
-#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Compiler.h"
 #include <optional>
 #include <utility>
 
 namespace clang {
 class Sema;
+class MultiLevelTemplateArgumentList;
 
-enum { ConstraintAlignment = 8 };
+/// \brief A normalized constraint, as defined in C++ [temp.constr.normal], is
+/// either an atomic constraint, a conjunction of normalized constraints or a
+/// disjunction of normalized constraints.
+
+struct NormalizedConstraint {
+
+  enum class ConstraintKind {
+    Atomic = 0,
+    ConceptId,
+    FoldExpanded,
+    Compound,
+  };
+
+  enum CompoundConstraintKind { CCK_Conjunction, CCK_Disjunction };
+  enum class FoldOperatorKind : unsigned int { And, Or };
+
+  using OccurenceList = llvm::SmallBitVector;
+
+  friend bool
+  substituteParameterMappings(Sema &S, NormalizedConstraint &N,
+                              const MultiLevelTemplateArgumentList &MLTAL,
+                              const ASTTemplateArgumentListInfo *ArgsAsWritten,
+                              TemplateParameterList *TemplateParams,
+                              const NamedDecl *D);
+
+protected:
+  using ExprOrConcept =
+      llvm::PointerUnion<const Expr *, const ConceptReference *>;
+
+  struct AtomicBits {
+    LLVM_PREFERRED_TYPE(ConstraintKind)
+    unsigned Kind : 5;
+    unsigned Placeholder : 1;
+    unsigned PackSubstitutionIndex : 26;
+    OccurenceList Indexes;
+    TemplateArgumentLoc *Args;
+    TemplateParameterList *ParamList;
+    ExprOrConcept ConstraintExpr;
+    const NamedDecl *ConstraintDecl;
+  };
+
+  struct FoldExpandedBits {
+    LLVM_PREFERRED_TYPE(ConstraintKind)
+    unsigned Kind : 5;
+    LLVM_PREFERRED_TYPE(FoldOperatorKind)
+    unsigned FoldOperator : 1;
+    unsigned Placeholder : 26;
+    OccurenceList Indexes;
+    TemplateArgumentLoc *Args;
+    TemplateParameterList *ParamList;
+    const Expr *Pattern;
+    const NamedDecl *ConstraintDecl;
+    NormalizedConstraint *Constraint;
+  };
+
+  struct ConceptIdBits : AtomicBits {
+    NormalizedConstraint *Sub;
+
+    // Only used for parameter mapping.
+    const ConceptSpecializationExpr *CSE;
+  };
+
+  struct CompoundBits {
+    LLVM_PREFERRED_TYPE(ConstraintKind)
+    unsigned Kind : 5;
+    LLVM_PREFERRED_TYPE(CompoundConstraintKind)
+    unsigned CCK : 1;
+    NormalizedConstraint *LHS;
+    NormalizedConstraint *RHS;
+  };
+
+  union {
+    AtomicBits Atomic;
+    FoldExpandedBits FoldExpanded;
+    ConceptIdBits ConceptId;
+    CompoundBits Compound;
+  };
+
+  ~NormalizedConstraint() {
+    if (getKind() != ConstraintKind::Compound)
+      Atomic.Indexes.llvm::SmallBitVector::~SmallBitVector();
+  }
+
+  NormalizedConstraint(const Expr *ConstraintExpr,
+                       const NamedDecl *ConstraintDecl,
+                       UnsignedOrNone PackIndex)
+      : Atomic{llvm::to_underlying(ConstraintKind::Atomic),
+               /*Placeholder=*/0,
+               PackIndex.toInternalRepresentation(),
+               /*Indexes=*/{},
+               /*Args=*/nullptr,
+               /*ParamList=*/nullptr,
+               ConstraintExpr,
+               ConstraintDecl} {}
+
+  NormalizedConstraint(const Expr *Pattern, FoldOperatorKind OpKind,
+                       NormalizedConstraint *Constraint,
+                       const NamedDecl *ConstraintDecl)
+      : FoldExpanded{llvm::to_underlying(ConstraintKind::FoldExpanded),
+                     llvm::to_underlying(OpKind),
+                     /*Placeholder=*/0,
+                     /*Indexes=*/{},
+                     /*Args=*/nullptr,
+                     /*ParamList=*/nullptr,
+                     Pattern,
+                     ConstraintDecl,
+                     Constraint} {}
+
+  NormalizedConstraint(const ConceptReference *ConceptId,
+                       const NamedDecl *ConstraintDecl,
+                       NormalizedConstraint *SubConstraint,
+                       const ConceptSpecializationExpr *CSE,
+                       UnsignedOrNone PackIndex)
+      : ConceptId{{llvm::to_underlying(ConstraintKind::ConceptId),
+                   /*Placeholder=*/0, PackIndex.toInternalRepresentation(),
+                   /*Indexes=*/{},
+                   /*Args=*/nullptr, /*ParamList=*/nullptr, ConceptId,
+                   ConstraintDecl},
+                  SubConstraint,
+                  CSE} {}
+
+  NormalizedConstraint(NormalizedConstraint *LHS, CompoundConstraintKind CCK,
+                       NormalizedConstraint *RHS)
+      : Compound{llvm::to_underlying(ConstraintKind::Compound), CCK, LHS, RHS} {
+  }
 
-struct alignas(ConstraintAlignment) AtomicConstraint {
-  const Expr *ConstraintExpr;
-  const NamedDecl *ConstraintDecl;
-  std::optional<ArrayRef<TemplateArgumentLoc>> ParameterMapping;
+  bool hasParameterMapping() const {
+    return getKind() != ConstraintKind::Compound
+                            && Atomic.Args != nullptr;
+  }
+
+  const OccurenceList &mappingOccurenceList() const {
+    assert(hasParameterMapping() && "Th...
[truncated]

@cor3ntin cor3ntin requested a review from a team as a code owner September 9, 2025 14:55
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 9, 2025
@cor3ntin cor3ntin force-pushed the corentin/use_normalization_for_satisfaction branch from c4e0ddb to 6f514b2 Compare September 9, 2025 15:29
Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for putting it up and the incredible work!

I left some notes, mostly are those that I didn't get around to yet (and I think they should be useful); so maybe we want to resolve them before we move forward.

Also let's not forget the release note :)

struct fltSemantics;
template <typename T, unsigned N> class SmallPtrSet;

template <> struct DenseMapInfo<llvm::FoldingSetNodeID> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe split it into an NFC PR? I'm not sure if downstream users want it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch is in desperate need of 'shrinking', I like the idea of splitting out any/everything we can.

@cor3ntin
Copy link
Contributor Author

cor3ntin commented Sep 9, 2025

Update: still needs cleaning but the test pass

These parameters should not affect subsumption.
This force us to maintain two occurence list for each atomic constraint.
const Expr *E, unsigned Depth, llvm::SmallBitVector &Used) {
MarkUsedTemplateParameterVisitor(Used, Depth, /*VisitDeclRefTypes=*/false)
.TraverseStmt(const_cast<Expr *>(E));
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return;

Comment on lines +1929 to +1933

SemaRef.MarkUsedTemplateParametersForSubsumptionParameterMapping(
static_cast<AtomicConstraint &>(N).getConstraintExpr(),
/*Depth=*/0, OccurringIndicesForSubsumption);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this is applicable to FoldExprs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some testing and i can't get it to failed without it, so I removed it!

@cor3ntin
Copy link
Contributor Author

cor3ntin commented Oct 2, 2025

There have not been any comments in a while - let's see if this sticks :)

@cor3ntin cor3ntin merged commit 9583b39 into llvm:main Oct 2, 2025
70 of 79 checks passed
@kstoimenov
Copy link
Contributor

Looks like it has a memory leak: https://lab.llvm.org/buildbot/#/builders/169/builds/15562. @cor3ntin please take a look.

@cor3ntin
Copy link
Contributor Author

cor3ntin commented Oct 2, 2025

Looks like it has a memory leak: https://lab.llvm.org/buildbot/#/builders/169/builds/15562. @cor3ntin please take a look.

Thanks. I reverted the patch shortly after landing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category HLSL HLSL Language Support libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang considers fold expanded constraint ambiguous when it shouldn't be Clang type substitution for concepts is too eager
8 participants